Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX]web_responsive: fixed the issue of file viewer not display in form view. #3122

Open
wants to merge 1 commit into
base: 17.0
Choose a base branch
from

Conversation

parvezqureshi
Copy link

@parvezqureshi parvezqureshi commented Mar 12, 2025

The attachments of customer invoices appear automatically (see screenshot). When we install oca/web/web_responsive, the attachments do not appear automatically (see screenshot). This is on a computer screen of course.

@OCA-git-bot
Copy link
Contributor

Hi @Tardo, @SplashS,
some modules you are maintaining are being modified, check this out!

@parvezqureshi parvezqureshi force-pushed the 17.0-fix-web_responsive branch from fd7d4bc to d4a58bf Compare March 12, 2025 07:13
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done on purpose. We don't want the preview to appear automatically. See #2745 #2795.

If you want to enable it, please include a system or user option.

@pedrobaeza pedrobaeza added this to the 17.0 milestone Mar 12, 2025
@parvezqureshi
Copy link
Author

@cvinh please review.

@cvinh
Copy link
Contributor

cvinh commented Mar 12, 2025

@cvinh please review.

Thanks @parvezqureshi please include a setup by user if someone wants attachment to open automatically or not... default =False (thanks to @pedrobaeza )

Please remove the screenshots, there are personal data in them... I think the reviewers understood the purpose

@parvezqureshi parvezqureshi force-pushed the 17.0-fix-web_responsive branch 6 times, most recently from 7fa550c to 19d2832 Compare March 12, 2025 09:18
@@ -70,6 +70,7 @@
<data>
<field name="action_id" position="after">
<field name="is_redirect_home" invisible="action_id" />
<field name="display_file_viewer" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it has an incorrect indentation

Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you :

  • improve the README explaining this new functionality
  • change your commit comment with [IMP] instead of [FIX]

@@ -31,6 +31,7 @@ class ResUsers(models.Model):
store=True,
readonly=False,
)
display_file_viewer = fields.Boolean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a help to explain what this field does

@cvinh
Copy link
Contributor

cvinh commented Mar 12, 2025

cc @vehi-invitu

Copy link

@vehi-invitu vehi-invitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setup is missing in base.view_users_form_simple_modif view

@hbrunn, what do you about this setup user ? It should be good solution for your PR
#3058

@parvezqureshi parvezqureshi force-pushed the 17.0-fix-web_responsive branch from 19d2832 to cabeea5 Compare March 13, 2025 05:17
@parvezqureshi parvezqureshi force-pushed the 17.0-fix-web_responsive branch from cabeea5 to 725185e Compare March 13, 2025 05:28
Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants